Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

treat bigint as string in msnodesqlv8 driver #1387

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dhensby
Copy link
Collaborator

@dhensby dhensby commented Apr 19, 2022

What this does:

BigInts should be treated as strings to ensure no precision loss

Related issues:

Attempt to fix #1385

Pre/Post merge checklist:

  • Update change log

@dhensby
Copy link
Collaborator Author

dhensby commented Apr 19, 2022

Well, interestingly, the tedious driver does not allow strings to be used for bigints and instead they must be set to varchars...

@blitzmann
Copy link
Contributor

Would there be an argument for using JS's BitInt type instead of a string? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt

mssql doesn't support Node versions < 10 according to changelog, and it's been around since 10.4

@dhensby
Copy link
Collaborator Author

dhensby commented Apr 21, 2022

Yep, definitely, but it's more about the underlying drivers than about this library. If they support either strings and/or BigInt, then we can use them.

@blitzmann
Copy link
Contributor

Ah that makes sense

@dhensby dhensby force-pushed the pulls/bigint-msnodesql branch 3 times, most recently from a7527cd to 5c27016 Compare April 28, 2022 21:07
@dhensby
Copy link
Collaborator Author

dhensby commented Sep 4, 2023

Well, interestingly, the tedious driver does not allow strings to be used for bigints and instead they must be set to varchars...

So this is the problem we have getting this merged, the tedious driver is the blocker

BigInt types will lose precision if they are above the MAX_SAFE_INTEGER
value. A work around for this is to use strings to represent the numbers
instead.
@dhensby
Copy link
Collaborator Author

dhensby commented Jun 18, 2024

So tedious introduced using native bigints for the bigint type in v18.2.0, which will be added in the next major release of this lib (imminent)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BigInt incorrectly cast to int with msnodesqlv8
2 participants